fix(backup): make EasyDash backup callback flow consistent#481
Open
mrrobot47 wants to merge 2 commits into
Open
fix(backup): make EasyDash backup callback flow consistent#481mrrobot47 wants to merge 2 commits into
mrrobot47 wants to merge 2 commits into
Conversation
The --dash-auth backup path could report success even when no backup remained, and could leave EasyDash with neither a success nor a failure callback. - Defer dash_backup_completed = true until after a confirmed success callback so the shutdown handler still fires the failure callback when the success callback fails and the upload is rolled back. - On success-callback failure, capture_error + EE::error after rollback so the exit code and message reflect that no backup remains, instead of falling through to EE::success. - Add a dash_callback_sent guard set at the actual send sites and checked by the shutdown handler so exactly one terminal callback (success XOR failure) is ever emitted. - Surface a failed rollback purge via capture_error + EE::error so the orphaned-remote-backup state propagates to the exit code and failure callback rather than being swallowed by a warning.
…load
Review follow-up to the dash-callback consistency fix.
- Give the callback-failure rollback its own error_code 4004; 4002 was
already in use for ERROR_TYPE_DATABASE ("Database backup failed"), and
error_code is shipped to EasyDash as a machine-readable field.
- When the rollback rclone purge also fails, force-overwrite the captured
dash error before re-capturing so the accurate FILESYSTEM 4003
"manually delete <path>" message wins over the caller's first-wins
optimistic "rolled back" message; otherwise EasyDash was told the
inverse of reality (rolled back) and never received the manual path.
- Demote the rollback-success notice from EE::success to EE::log since it
always precedes a terminal EE::error.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns EasyDash (--dash-auth) backup outcomes so the CLI exit status and console output match what EasyDash is told via callbacks, especially around callback failures and rollback behavior.
Changes:
- Delays marking
dash_backup_completeduntil after a confirmed EasyDash success callback, ensuring the shutdown failure callback is not accidentally suppressed. - Introduces
dash_callback_sentas a guard so exactly one terminal callback (success or failure) is emitted. - Improves rollback error reporting so a rollback purge failure reports an accurate “manual delete” message/code to EasyDash and avoids misleading success logging during an overall failure.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the EasyDash (
--dash-auth) backup callback flow consistent, so the command's exit status, the console message, and the callback EasyDash receives all agree with what actually happened.Bugs fixed
rollback_failed_backup()deleted the just-uploaded backup, yet the command still printedEE::success('Backup created successfully.')and exited 0. It now captures the error, rolls back, andEE::errors — accurate non-zero exit + message.dash_backup_completedwas settruebefore the success callback, so a failed callback also suppressed the shutdown failure callback → EasyDash was told neither success nor failure. It is now set only after a confirmed success callback (and on the non-dash path), so the failure callback fires.dash_callback_sentguard, set at the real send sites and checked by the shutdown handler — exactly one terminal callback (success XOR failure), never zero, never double.rclone purgealso fails (the orphan survives on remote), EasyDash now receives the accurate "manually delete<path>" message (code4003) instead of the optimistic "rolled back" one —capture_erroris first-wins, so the purge-failure branch force-overwrites it.Also: the new callback-failure uses code
4004(avoids colliding with the existing4002"Database backup failed"); the rollback-success notice is demotedEE::success→EE::log(it always precedes a terminal error).Verification
Reviewed by two independent reviewers (correctness + design lenses). Both confirmed exactly one terminal callback across every path (success/callback-fail/purge-fail/pre-upload-abort/fatal/non-dash), verified via truth table + a PHP simulation of the exit/shutdown/
capture_errorbehavior. Their two must-fixes (the4002collision and the orphan-payload override) are incorporated.